-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
support for legacy content collections removed #12376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
|
|
||
| If you have content collections errors after upgrading to v6, use the following list to help you identify and upgrade any legacy features that may exist in your code. | ||
|
|
||
| ##### If you have... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add the error messages in here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this could be helpful, or at least the error code so someone can easily identify the details to expand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea! Would the implementation PR be a comprehensive list of all the ones they might encounter?
I'm seeing:
- 'LegacyContentConfigError'
- 'ContentCollectionMissingLoader'
- 'ContentCollectionInvalidType'
What we did for v5 was mock up the new errors / config in the v5 branch (duplicating what would be generated from the JSDoc), so we should probabably manually recreate those errors in this PR, too. If they end up not being identical, then a ci PR will be generated to align them.
Just copied the entire jsdoc from entries here for now
- @docs
- @message
- Example error message:
- Found legacy content config file in "src/content/config.ts". Please move this file to "src/content.config.ts" and ensure each collection has a loader defined.
- @description
- A legacy content config file was found. Move the file to
src/content.config.tsand update any collection definitions if needed. - See the Astro 6 migration guide for more information.
*/
export const LegacyContentConfigError = {
name: 'LegacyContentConfigError',
title: 'Legacy content config file found.',
message: (filename: string) =>
Found legacy content config file in "${filename}". Please move this file to "src/content.config.${filename.split('.').at(-1)}" and ensure each collection has a loader defined.,
hint: 'See https://docs.astro.build/en/guides/upgrade-to/v6/#removed-legacy-content-collections for more information on updating collections.',
} satisfies ErrorData;
/**
- @docs
- @message
- Example error message:
- Collections must have a
loaderdefined. Check your collection definitions in your content config file. - @description
- A content collection is missing a
loaderdefinition. Make sure that each collection in your content config file has aloader. - See the Content collections documentation for more information.
*/
export const ContentCollectionMissingLoader = {
name: 'ContentCollectionMissingLoader',
title: 'Content collection is missing a loader definition.',
message: (file = 'your content config file') =>
Collections must have a \loader` defined. Check your collection definitions in ${file}.`,
hint: 'See https://docs.astro.build/en/guides/content-collections/ for more information on content loaders and https://docs.astro.build/en/guides/upgrade-to/v6/#removed-legacy-content-collections for more information on migrating from legacy collections.',
} satisfies ErrorData;
/**
- @docs
- @message
- Example error message:
- Invalid collection type "data". Remove the type from your collection definition in your content config file.
- @description
- Content collections should no longer have a
typefield. Remove this field from your content config file. - See the Astro 6 migration guide for more information.
*/
export const ContentCollectionInvalidType = {
name: 'ContentCollectionInvalidType',
title: 'Content collection has an invalid type field.',
message: (type: string, file = 'your content config file') =>
Invalid collection type "${type}". Remove the type from your collection definition in ${file}.,
hint: 'See https://docs.astro.build/en/guides/upgrade-to/v6/#removed-legacy-content-collections for more information on migrating from legacy collections.',
} satisfies ErrorData;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also some places where we just log a warning rather than throwing an error:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add a more searchable name there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't get to the spot in that link directly from the link, because GitHub can't handle the PR. 😅 Which file are warnings included in so I can get there myself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've updated the text to refer to both error messages and warnings and we can do further edits on top of that!
I'm not sure they all need to be spelled out here, since in theory those should have their own explanations and guidance. To avoid this content being overwhelming, I think just a mention that "if you get warnings/errors, these are the likely places to check" is fine here! If people are not getting these errors/warnings, no need to be spelling them out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just meant adding the specific codes such as ContentCollectionMissingLoader to the section, to make each section more searchable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I think I get it. Let me cook!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, here's my current draft to address this:
@ascorbic I didn't notice specific errors (either in your removing legacy support PR, nor existing ones) for the situation where:
- there is NO content config file at all
- they are not importing and using the
renderfunction (and are instead calling it on the entry)
So, those two don't have associated errors mentioned here. Do we have such errors, and if not, should we?
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small nit, otherwise I think this format looks great and makes sense!
Co-authored-by: Matt Kane <[email protected]>
src/content/docs/en/reference/errors/content-collection-invalid-type.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/en/reference/errors/content-collection-missing-loader.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/en/reference/errors/legacy-content-config-error.mdx
Outdated
Show resolved
Hide resolved
src/content/docs/en/reference/errors/legacy-content-config-error.mdx
Outdated
Show resolved
Hide resolved
|
Made some changes and resolved some conversations with explanations not to shut down further editing, just to tidy up and so we can work from something fresh now! |
src/content/docs/en/reference/errors/content-collection-missing-loader.mdx
Outdated
Show resolved
Hide resolved
|
I think all the pieces are here, and I did some minor polishing. Time to review again! Note that there ARE error messages in the NB: as noted above, any updating of the actual content collections guide page itself will come with the stabilization of Live Collections. Since non-legacy (current, non-live) collections are the main game in town, our current content collections page needs no updating to correspond to the loss of legacy collections. |
ArmandPhilippot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, I left a few comments but those are mostly nitpicks!
Co-authored-by: Armand Philippot <[email protected]>
|
Going to consider this one acceptable enough to merge! I searched, but couldn't find any related error messages to not having a config file, nor using the older Still lots of time to make updates if we notice them, but this way, this content is merged and we can link to it in the preview of the upgrade guide. |
This adds "Removed: content collections" to the v6 upgrade guide
Direct deploy preview: https://deploy-preview-12376--astro-docs-2.netlify.app/en/guides/upgrade-to/v6/#removed-legacy-content-collections
Implementation PR #14407
This PR should include
entry.render()Follow-up/concurrent tasks:
Notes:
astro:contentreference updated needed: v5 already only documents the newest API